Skip to content

fix: score each sequence independently in compute_pwm - #38

Merged
aviezerl merged 1 commit into
masterfrom
fix/compute-pwm-variable-length
Jul 13, 2026
Merged

fix: score each sequence independently in compute_pwm#38
aviezerl merged 1 commit into
masterfrom
fix/compute-pwm-variable-length

Conversation

@aviezerl

Copy link
Copy Markdown
Contributor

Problem

Reported by Tamar: compute_pwm() gives a sequence a different score depending on the other sequences in the batch when sequences have different lengths.

compute_pwm(c("AACCAGCT", "AACCAGCTTCATTCATGCAAATTAGGGGACCACAGGAGTCCC"),
            pssm = motif_pssms$oct4, func = "max", bidirect = TRUE)
# 2nd sequence -> -14.62

compute_pwm(c("AACCAGCTTCATTCATGCAAATTAGGGGACCACAGGAGTCCC"),
            pssm = motif_pssms$oct4, func = "max", bidirect = TRUE)
# same sequence alone -> -5.58

Root cause

The motif-scan window was capped to nchar(sequences[1]) for the whole batch (passed to the C++ spat_max, and the flat spatial bin was sized the same way). A sequence longer than the first was therefore only scanned over its first nchar(sequences[1]) starting positions, missing motif hits further along. A shorter first sequence (8 bp) truncated the 42 bp Oct4 sequence to its first 8 positions.

The C++ core (shared with misha's DnaPSSM) already clamps the scan per sequence to end - motif_len; the cap was purely an artifact of the R wrapper deriving one window from sequences[1].

Fix

Use max(nchar(sequences)) for both the scan range and the flat spatial bin. Each sequence is then scanned to its own end, independently of its batch companions. Equal-length batches (the regression / screen_kmers path) are byte-identical since max == first.

The sibling functions (compute_local_pwm, screen_local_pwm, mask_sequences_by_pwm) carry the same nchar(sequences[1]) line but pass motif-length windows to the scan, so the range is inert there - left unchanged.

Test

Added a regression test in test-compute_pwm.R asserting a sequence's score is batch-position-independent (both max and logSumExp). Fails before the fix, passes after; existing PWM tests unchanged.

https://claude.ai/code/session_01PK3qefBGDoBwd9w5226FEq

compute_pwm() capped the motif-scan window to nchar(sequences[1]) for the
whole batch, so a sequence longer than the first was only scanned over its
first few positions and could miss its real motif hit - making a sequence's
score depend on its batch companions.

Use max(nchar(sequences)) for both the scan range and the flat spatial bin;
the C++ scan already clamps per-sequence to each sequence's own end, so every
sequence is now scored on its full length. Equal-length batches (regression /
screen_kmers) are unchanged since max == first.

Claude-Session: https://claude.ai/code/session_01PK3qefBGDoBwd9w5226FEq
@aviezerl
aviezerl merged commit 6023497 into master Jul 13, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant